deploy/docker/fs/opt/appsmith/utils/bin/export_db.js (1)

Line range hint 43-65: Improve error handling and application state management

The error handling could be more specific and ensure proper application restart in all scenarios.

+  const DatabaseExportError = class extends Error {
+    constructor(message, cause) {
+      super(message);
+      this.cause = cause;
+      this.name = 'DatabaseExportError';
+    }
+  };
+
   try {
     check_supervisord_status_cmd = '/usr/bin/supervisorctl >/dev/null 2>&1 ';
     shell.exec(check_supervisord_status_cmd, function (code) {
       if (code > 0) {
-        shell.echo('application is not running, starting supervisord');
+        throw new DatabaseExportError('Supervisord is not running');
         shell.exec('/usr/bin/supervisord');
       }
     });

deploy/docker/fs/opt/appsmith/utils/bin/utils.js (1)

Add PostgreSQL support and remove MongoDB dependencies in utils.js

The preprocessMongoDBURI function is still present and PostgreSQL support is not implemented.

• deploy/docker/fs/opt/appsmith/utils/bin/utils.js (Line 125)

🔗 Analysis chain

Line range hint 1-201: Verify PostgreSQL support in utility functions

Several utility functions in this file were originally designed for MongoDB. Let's verify they work correctly with PostgreSQL.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for PostgreSQL-specific handling in the codebase
rg -l 'postgres|postgresql' --type js

# Check for any potential MongoDB-specific operations that might need PostgreSQL equivalents
ast-grep --pattern 'MongoClient|mongoose|mongodb' 

Length of output: 1305

---

Script:

#!/bin/bash
# Verify PostgreSQL support within utils.js
ast-grep --pattern 'PostgreSQL|pg' deploy/docker/fs/opt/appsmith/utils/bin/utils.js

# Check if MongoDB functions are still being used or referenced in utils.js
ast-grep --pattern 'preprocessMongoDBURI' deploy/docker/fs/opt/appsmith/utils/bin/utils.js

Length of output: 279

deploy/docker/fs/opt/appsmith/utils/bin/backup.test.js (1)

59-67: Add test cases for edge cases in PostgreSQL connection string

While the basic test case looks good, consider adding tests for:

• Invalid connection strings
• Connection strings with special characters
• Different PostgreSQL connection string formats

deploy/docker/fs/opt/appsmith/utils/bin/import_db.js (2)

38-46: Avoid redundant utils.getDburl() calls

Pass dbUrl as a parameter to get_table_or_collection_len to prevent multiple calls.

Apply this diff:

- function get_table_or_collection_len() {
+ function get_table_or_collection_len(dbUrl) {

Update the call at line 63:

- const collectionsLen = get_table_or_collection_len();
+ const collectionsLen = get_table_or_collection_len(dbUrl);

---

12-16: Handle unsupported database types

Add an else clause to handle cases where the database type is neither MongoDB nor PostgreSQL.

Apply this diff:

  } else if (dbUrl.startsWith('postgresql')) {
    restore_postgres_db(dbUrl);
+ } else {
+   console.error(`Unsupported database type: ${dbUrl}`);
+   process.exit(1);
  }

deploy/docker/fs/opt/appsmith/utils/bin/backup.js (1)

128-133: Store database URL in a variable to avoid multiple function calls

Currently, utils.getDburl() is called multiple times. Consider storing the result in a variable for efficiency.

Apply this diff to implement the change:

+      const dbUrl = utils.getDburl();
-      if (utils.getDburl().startsWith('mongodb')) {
+      if (dbUrl.startsWith('mongodb')) {
-        await executeMongoDumpCMD(destFolder, utils.getDburl())
+        await executeMongoDumpCMD(destFolder, dbUrl)
-      } else if (utils.getDburl().startsWith('postgresql')) {
+      } else if (dbUrl.startsWith('postgresql')) {
-        await executePostgresDumpCMD(destFolder, utils.getDburl());
+        await executePostgresDumpCMD(destFolder, dbUrl);
       }

deploy/docker/fs/opt/appsmith/utils/bin/restore.js (1)

64-65: Use consistent methods for database type checks

For consistency, use dbUrl.startsWith('postgresql') instead of dbUrl.includes('postgresql') when checking for PostgreSQL URLs.

Configuration used: .coderabbit.yaml
Review profile: CHILL

- const cmd = `pg_restore -U postgres -d appsmith --verbose --clean ${Constants.RESTORE_PATH}/${Constants.DUMP_FILE_NAME}`;
+ const dbName = utils.getDbNameFromUrl(dbUrl);
+ const cmd = `pg_restore -U postgres -d ${dbName} --verbose --clean ${Constants.RESTORE_PATH}/${Constants.DUMP_FILE_NAME}`;

- restore_postgres_db = (dbUrl) => {
+ const restore_postgres_db = (dbUrl) => {

Configuration used: .coderabbit.yaml
Review profile: CHILL

Learnt from: abhvsn
PR: appsmithorg/appsmith#37326
File: deploy/docker/fs/opt/appsmith/utils/bin/backup.js:169-171
Timestamp: 2024-11-13T09:39:34.416Z
Learning: In `backup.js`, when using `pg_dump` in the `executePostgresDumpCMD` function, the `--dbname=` option is not mandatory; the PostgreSQL URL can be passed directly.

Configuration used: .coderabbit.yaml
Review profile: CHILL

Learnt from: abhvsn
PR: appsmithorg/appsmith#37326
File: deploy/docker/fs/opt/appsmith/utils/bin/backup.js:169-171
Timestamp: 2024-11-13T09:39:34.416Z
Learning: In `backup.js`, when using `pg_dump` in the `executePostgresDumpCMD` function, the `--dbname=` option is not mandatory; the PostgreSQL URL can be passed directly.

Learnt from: abhvsn
PR: appsmithorg/appsmith#37326
File: deploy/docker/fs/opt/appsmith/utils/bin/export_db.js:11-17
Timestamp: 2024-11-13T09:41:48.119Z
Learning: In the `export_db.js` script located at `deploy/docker/fs/opt/appsmith/utils/bin/export_db.js`, it's acceptable to use simple string-based detection (e.g., `dbUrl.startsWith('mongodb')`) for determining the database type because the database URL is restrictive, and misconfiguration will cause the server to fail to start up. Therefore, using a URL parsing library for database URL detection is unnecessary in this context.

Learnt from: abhvsn
PR: appsmithorg/appsmith#37326
File: deploy/docker/fs/opt/appsmith/utils/bin/backup.js:169-171
Timestamp: 2024-11-13T09:39:34.416Z
Learning: In `backup.js`, when using `pg_dump` in the `executePostgresDumpCMD` function, the `--dbname=` option is not mandatory; the PostgreSQL URL can be passed directly.

-    const collectionsLen = get_table_or_collection_len();
+    const dbUrl = utils.getDburl();
+    const collectionsLen = get_table_or_collection_len(dbUrl);

 restore_mongo_db = (dbUrl) => {
+  if (!shell.test('-f', `${Constants.RESTORE_PATH}/${Constants.DUMP_FILE_NAME}`)) {
+    throw new Error('MongoDB dump file not found');
+  }
+  const sanitizedUrl = dbUrl.replace(/[;&|`$]/g, '');
-  const cmd = `mongorestore --uri='${dbUrl}' --drop --archive='${Constants.RESTORE_PATH}/${Constants.DUMP_FILE_NAME}' --gzip`;
+  const cmd = `mongorestore --uri='${sanitizedUrl}' --drop --archive='${Constants.RESTORE_PATH}/${Constants.DUMP_FILE_NAME}' --gzip`;
-  shell.exec(cmd);
+  const result = shell.exec(cmd);
+  if (result.code !== 0) {
+    throw new Error(`MongoDB restore failed: ${result.stderr}`);
+  }
 }

   const dbUrl = utils.getDburl();
   // Check the DB url
+  if (!dbUrl) {
+    throw new Error('Database URL is not configured');
+  }
   if (dbUrl.startsWith('mongodb')) {
     await executeMongoDumpCMD(destFolder, dbUrl);
   } else if (dbUrl.startsWith('postgresql')) {
     await executePostgresDumpCMD(destFolder, dbUrl);
+  } else {
+    throw new Error(`Unsupported database type in URL: ${dbUrl}`);
   }

deploy/docker/fs/opt/appsmith/utils/bin/backup.js (1)

152-152: Add version validation in manifest

Ensure version is not null before creating manifest.

-  const manifest_data = { "appsmithVersion": version, "dbName": utils.getDatabaseNameFromDBURI(utils.getDburl()) }
+  if (!version) {
+    throw new Error('Unable to determine Appsmith version');
+  }
+  const manifest_data = { 
+    "appsmithVersion": version, 
+    "dbName": utils.getDatabaseNameFromDBURI(utils.getDburl()),
+    "backupType": utils.getDburl().startsWith('mongodb') ? 'mongodb' : 'postgresql'
+  }

deploy/docker/fs/opt/appsmith/utils/bin/backup.test.js (2)

58-66: Add error handling test cases for PostgreSQL dump command

While the happy path is covered, consider adding test cases for error scenarios such as invalid connection strings or failed dump operations.

Example test case to add:

test('Test postgresdump CMD with invalid URI', async () => {
  var dest = '/dest'
  var invalidURI = 'invalid://uri'
  await expect(backup.executePostgresDumpCMD(dest, invalidURI)).rejects.toThrow()
})

---

266-276: Add more edge cases for PostgreSQL URI parsing

Consider adding test cases for:

• URIs with special characters in database name
• URIs with multiple query parameters
• URIs with non-standard ports

Example test cases to add:

test('Get DB name from PostgreSQL URI with special characters', async () => {
  var pg_uri = "postgresql://user:password@host:5432/postgres-db_1"
  var expectedDBName = 'postgres-db_1'
  expect(utils.getDatabaseNameFromDBURI(pg_uri)).toEqual(expectedDBName)
})

test('Get DB name from PostgreSQL URI with multiple query params', async () => {
  var pg_uri = "postgresql://user:password@host:5432/postgres_db?sslmode=verify-full&target_session_attrs=read-write"
  var expectedDBName = 'postgres_db'
  expect(utils.getDatabaseNameFromDBURI(pg_uri)).toEqual(expectedDBName)
})

Configuration used: .coderabbit.yaml
Review profile: CHILL

Learnt from: abhvsn
PR: appsmithorg/appsmith#37326
File: deploy/docker/fs/opt/appsmith/utils/bin/backup.js:169-171
Timestamp: 2024-11-13T09:39:34.416Z
Learning: In `backup.js`, when using `pg_dump` in the `executePostgresDumpCMD` function, the `--dbname=` option is not mandatory; the PostgreSQL URL can be passed directly.

Learnt from: abhvsn
PR: appsmithorg/appsmith#37326
File: deploy/docker/fs/opt/appsmith/utils/bin/backup.js:169-171
Timestamp: 2024-11-13T09:39:34.416Z
Learning: In `backup.js`, when using `pg_dump` in the `executePostgresDumpCMD` function, the `--dbname=` option is not mandatory; the PostgreSQL URL can be passed directly.

Learnt from: abhvsn
PR: appsmithorg/appsmith#37326
File: deploy/docker/fs/opt/appsmith/utils/bin/export_db.js:11-17
Timestamp: 2024-11-13T09:41:48.119Z
Learning: In the `export_db.js` script located at `deploy/docker/fs/opt/appsmith/utils/bin/export_db.js`, it's acceptable to use simple string-based detection (e.g., `dbUrl.startsWith('mongodb')`) for determining the database type because the database URL is restrictive, and misconfiguration will cause the server to fail to start up. Therefore, using a URL parsing library for database URL detection is unnecessary in this context.

Learnt from: abhvsn
PR: appsmithorg/appsmith#37326
File: deploy/docker/fs/opt/appsmith/utils/bin/backup.js:169-171
Timestamp: 2024-11-13T09:39:34.416Z
Learning: In `backup.js`, when using `pg_dump` in the `executePostgresDumpCMD` function, the `--dbname=` option is not mandatory; the PostgreSQL URL can be passed directly.

+const sanitizeUrl = (url) => url.replace(/[$&;`|*?~<>^()!{}[\]]/g, '\\$&');
+const sanitizedDbUrl = sanitizeUrl(dbUrl);
 if (dbUrl.startsWith('mongodb')) {
-  cmd = `mongodump --uri='${dbUrl}' --archive='${Constants.BACKUP_PATH}/${Constants.DUMP_FILE_NAME}' --gzip`;
+  cmd = `mongodump --uri='${sanitizedDbUrl}' --archive='${Constants.BACKUP_PATH}/${Constants.DUMP_FILE_NAME}' --gzip`;
 } else if (dbUrl.startsWith('postgresql')) {
-  cmd = `pg_dump ${dbUrl} -n appsmith -Fc -f '${Constants.BACKUP_PATH}/${Constants.DUMP_FILE_NAME}'`;
+  cmd = `pg_dump '${sanitizedDbUrl}' -n appsmith -Fc -f '${Constants.BACKUP_PATH}/${Constants.DUMP_FILE_NAME}'`;